Skip to content

[AMDGPU] Fix an invalid cast in AMDGPULateCodeGenPrepare::visitLoadInst #122494

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 13, 2025
Merged

[AMDGPU] Fix an invalid cast in AMDGPULateCodeGenPrepare::visitLoadInst #122494

merged 1 commit into from
Jan 13, 2025

Conversation

@shiltian
Copy link
Contributor

@shiltian shiltian commented Jan 10, 2025 •
edited by arsenm
Loading

Fixes: SWDEV-507695

@shiltian Graphite App
Copy link
Contributor Author

shiltian commented Jan 10, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@llvmbot
Copy link
Member

llvmbot commented Jan 10, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Shilei Tian (shiltian)

Changes

Fixes SWDEV-507695.

Full diff: https://github.com/llvm/llvm-project/pull/122494.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp (+8-2)
  • (added) llvm/test/CodeGen/AMDGPU/invalid-cast-load-i1.ll (+43)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp b/llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp
index 830b50307f837e..589a2ef8b54f09 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp
@@ -464,8 +464,14 @@ bool AMDGPULateCodeGenPrepare::visitLoadInst(LoadInst &LI) {
   NewLd->setMetadata(LLVMContext::MD_range, nullptr);
 
   unsigned ShAmt = Adjust * 8;
-  auto *NewVal = IRB.CreateBitCast(
-      IRB.CreateTrunc(IRB.CreateLShr(NewLd, ShAmt), IntNTy), LI.getType());
+  Value *NewVal = nullptr;
+  if (DL.typeSizeEqualsStoreSize(LI.getType())) {
+    NewVal = IRB.CreateBitCast(
+        IRB.CreateTrunc(IRB.CreateLShr(NewLd, ShAmt), IntNTy), LI.getType());
+  } else {
+    assert(DL.getTypeSizeInBits(LI.getType()) < LdBits);
+    NewVal = IRB.CreateTrunc(IRB.CreateLShr(NewLd, ShAmt), LI.getType());
+  }
   LI.replaceAllUsesWith(NewVal);
   DeadInsts.emplace_back(&LI);
 
diff --git a/llvm/test/CodeGen/AMDGPU/invalid-cast-load-i1.ll b/llvm/test/CodeGen/AMDGPU/invalid-cast-load-i1.ll
new file mode 100644
index 00000000000000..e842a70b0aed65
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/invalid-cast-load-i1.ll
@@ -0,0 +1,43 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx90a %s -o - | FileCheck %s
+
+@g = global ptr null
+
+define amdgpu_kernel void @load_idx_idy(ptr addrspace(4) %disp) {
+; CHECK-LABEL: load_idx_idy:
+; CHECK:       ; %bb.0: ; %entry
+; CHECK-NEXT:    s_load_dword s2, s[4:5], 0x4
+; CHECK-NEXT:    s_load_dwordx2 s[0:1], s[8:9], 0x0
+; CHECK-NEXT:    v_mov_b32_e32 v0, 0
+; CHECK-NEXT:    s_waitcnt lgkmcnt(0)
+; CHECK-NEXT:    s_lshr_b32 s2, s2, 16
+; CHECK-NEXT:    s_bfe_i64 s[2:3], s[2:3], 0x10000
+; CHECK-NEXT:    s_lshl_b64 s[2:3], s[2:3], 6
+; CHECK-NEXT:    s_add_u32 s0, s0, s2
+; CHECK-NEXT:    s_addc_u32 s1, s1, s3
+; CHECK-NEXT:    global_load_ubyte v2, v0, s[0:1] offset:4
+; CHECK-NEXT:    s_getpc_b64 s[0:1]
+; CHECK-NEXT:    s_add_u32 s0, s0, g@gotpcrel32@lo+4
+; CHECK-NEXT:    s_addc_u32 s1, s1, g@gotpcrel32@hi+12
+; CHECK-NEXT:    s_load_dwordx2 s[0:1], s[0:1], 0x0
+; CHECK-NEXT:    s_waitcnt lgkmcnt(0)
+; CHECK-NEXT:    v_pk_mov_b32 v[0:1], s[0:1], s[0:1] op_sel:[0,1]
+; CHECK-NEXT:    s_waitcnt vmcnt(0)
+; CHECK-NEXT:    flat_store_byte v[0:1], v2
+; CHECK-NEXT:    s_endpgm
+entry:
+  %disp1 = tail call ptr addrspace(4) @llvm.amdgcn.dispatch.ptr()
+  %gep_y = getelementptr i8, ptr addrspace(4) %disp1, i64 6
+  %L = load i1, ptr addrspace(4) %gep_y, align 1
+  %idxprom = sext i1 %L to i64
+  %gep0 = getelementptr <32 x i16>, ptr addrspace(4) %disp, i64 %idxprom
+  %gep1 = getelementptr i8, ptr addrspace(4) %gep0, i64 4
+  %L1 = load i8, ptr addrspace(4) %gep1
+  store i8 %L1, ptr @g
+  ret void
+}
+
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare noundef nonnull align 4 ptr addrspace(4) @llvm.amdgcn.dispatch.ptr() #0
+
+attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }

shiltian
shiltian commented Jan 10, 2025
View reviewed changes
arsenm
arsenm reviewed Jan 11, 2025
View reviewed changes
@shiltian shiltian force-pushed the users/shiltian/invalid-cast-in-visitLoadInst branch from abc1056 to 87817df Compare January 13, 2025 03:34
arsenm
arsenm approved these changes Jan 13, 2025
View reviewed changes
llvm/test/CodeGen/AMDGPU/invalid-cast-load-i1.ll
; CHECK-NEXT: flat_store_byte v[0:1], v2
; CHECK-NEXT: s_endpgm
entry:
%disp1 = tail call ptr addrspace(4) @llvm.amdgcn.dispatch.ptr()
Copy link
Contributor

@arsenm arsenm Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be able to use a pointer argument and avoid the special intrinsic. Also can you merge this in with an existing test for this function?

Copy link
Contributor Author

@shiltian shiltian Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be able to use a pointer argument and avoid the special intrinsic.

The current version was from llvm-reduce. I did try to get rid of the intrinsic but failed to do so, then I didn't dig more into that.

Also can you merge this in with an existing test for this function?

I'm not sure which existing test can be a good fit for this one.

Copy link
Contributor

@arsenm arsenm Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably missed adding the dereferenceable and align attributes to the pointer, which are implied by the intrinsic declaration

@shiltian shiltian merged commit f15da5f into main Jan 13, 2025
6 of 8 checks passed
@shiltian shiltian deleted the users/shiltian/invalid-cast-in-visitLoadInst branch January 13, 2025 04:40
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 13, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-expensive-checks-debian running on gribozavr4 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/11861

Here is the relevant piece of the build log for the reference 
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Input file: /b/1/llvm-clang-x86_64-expensive-checks-debian/build/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.dSYM
Output file (aarch64): /b/1/llvm-clang-x86_64-expensive-checks-debian/build/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.default.gSYM
Loaded 3 functions from DWARF.
Loaded 3 functions from symbol table.
warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000030
addr=0x0000000000000248, file=  2, line=  5
addr=0x0000000000000254, file=  2, line=  7
addr=0x0000000000000258, file=  2, line=  9
addr=0x000000000000025c, file=  2, line=  8
addr=0x0000000000000260, file=  2, line= 11
addr=0x0000000000000264, file=  2, line= 10
addr=0x0000000000000268, file=  2, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000047
addr=0x0000000000000248, file=  3, line=  5
addr=0x0000000000000254, file=  3, line=  7
addr=0x0000000000000258, file=  3, line=  9
addr=0x000000000000025c, file=  3, line=  8
addr=0x0000000000000260, file=  3, line= 11
addr=0x0000000000000264, file=  3, line= 10
addr=0x0000000000000268, file=  3, line=  6


warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000047
addr=0x0000000000000248, file=  3, line=  5
addr=0x0000000000000254, file=  3, line=  7
addr=0x0000000000000258, file=  3, line=  9
addr=0x000000000000025c, file=  3, line=  8
addr=0x0000000000000260, file=  3, line= 11
addr=0x0000000000000264, file=  3, line= 10
addr=0x0000000000000268, file=  3, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000001
addr=0x0000000000000248, file=  1, line=  5
addr=0x0000000000000254, file=  1, line=  7
addr=0x0000000000000258, file=  1, line=  9
addr=0x000000000000025c, file=  1, line=  8
addr=0x0000000000000260, file=  1, line= 11
addr=0x0000000000000264, file=  1, line= 10
...

kazutakahirata pushed a commit to kazutakahirata/llvm-project that referenced this pull request Jan 13, 2025
@shiltian @kazutakahirata
[AMDGPU] Fix an invalid cast in `AMDGPULateCodeGenPrepare::visitLoadI…
30e9022 
…nst` (llvm#122494)

Fixes: SWDEV-507695
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arsenm arsenm arsenm approved these changes

@jayfoad jayfoad Awaiting requested review from jayfoad

@Sisyph Sisyph Awaiting requested review from Sisyph

@rampitec rampitec Awaiting requested review from rampitec

Assignees

No one assigned

Labels

backend:AMDGPU

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@shiltian @llvmbot @llvm-ci @arsenm